Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make yarn rebuild:electron fail explicitly on Windows #6538

Merged
merged 1 commit into from
Nov 19, 2019
Merged

Make yarn rebuild:electron fail explicitly on Windows #6538

merged 1 commit into from
Nov 19, 2019

Conversation

paul-marechal
Copy link
Member

What it does

Before this commit, the yarn rebuild:electron used to fail silently.
We now get a proper output and exit code.

How to test

On Windows, try to run yarn rebuild:electron. Apparently there is a failure due to some http 403 error when downloading node-gyp build dependencies.

Review checklist

Reminder for reviewers

@paul-marechal
Copy link
Member Author

Current error I am having:

D:\sources\theia>yarn rebuild:electron
yarn run v1.16.0
$ theia rebuild:electron
Processing @theia/node-pty
Processing nsfw
Processing native-keymap
Processing find-git-repositories
× Rebuild Failed
Uncaught Exception:  Error: gyp info it worked if it ends with ok
gyp info using node-gyp@5.0.5
gyp info using node@10.15.3 | win32 | x64
gyp info find Python using Python version 2.7.16 found at "D:\windows\programs\Python27\python.exe"
gyp http GET https://electronjs.org/headers/v4.2.12/node-v4.2.12-headers.tar.gz
gyp http 200 https://electronjs.org/headers/v4.2.12/node-v4.2.12-headers.tar.gz
gyp http GET https://electronjs.org/headers/v4.2.12/SHASUMS256.txt
gyp http GET https://electronjs.org/headers/v4.2.12/win-x86/node.lib
gyp http GET https://electronjs.org/headers/v4.2.12/win-x64/node.lib
gyp http GET https://electronjs.org/headers/v4.2.12/win-arm64/node.lib
gyp http 200 https://electronjs.org/headers/v4.2.12/SHASUMS256.txt
gyp http 403 https://electronjs.org/headers/v4.2.12/win-arm64/node.lib
gyp WARN install got an error, rolling back install
gyp ERR! configure error
gyp ERR! stack Error: 403 status code downloading arm64 node.lib
gyp ERR! stack     at Request.<anonymous> (D:\sources\theia\node_modules\electron-rebuild\node_modules\node-gyp\lib\install.js:335:22)
gyp ERR! stack     at Request.emit (events.js:194:15)
gyp ERR! stack     at Request.onRequestResponse (D:\sources\theia\node_modules\request\request.js:1066:10)
gyp ERR! stack     at ClientRequest.emit (events.js:189:13)
gyp ERR! stack     at HTTPParser.parserOnIncomingClient [as onIncoming] (_http_client.js:556:21)
gyp ERR! stack     at HTTPParser.parserOnHeadersComplete (_http_common.js:109:17)
gyp ERR! stack     at TLSSocket.socketOnData (_http_client.js:442:20)
gyp ERR! stack     at TLSSocket.emit (events.js:189:13)
gyp ERR! stack     at addChunk (_stream_readable.js:284:12)
gyp ERR! stack     at readableAddChunk (_stream_readable.js:265:11)
gyp ERR! System Windows_NT 10.0.17763
gyp ERR! command "D:\\windows\\programs\\nodejs\\node.exe" "D:\\sources\\theia\\node_modules\\electron-rebuild\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild" "--target=4.2.12" "--arch=x64" "--dist-url=https://electronjs.org/headers" "--build-from-source"
gyp ERR! cwd D:\sources\theia\node_modules\@theia\node-pty
gyp ERR! node -v v10.15.3
gyp ERR! node-gyp -v v5.0.5
gyp ERR! not ok

Failed with exit code: 1

@paul-marechal paul-marechal changed the title Make yarn:electron fail explicitly on Windows Make yarn rebuild:electron fail explicitly on Windows Nov 13, 2019
@paul-marechal paul-marechal mentioned this pull request Nov 13, 2019
Closed
@paul-marechal
Copy link
Member Author

Even if a new error is brought to light by this PR, I think we should merge it sooner than later. Not seeing errors doesn't mean there aren't errors. Especially when the Electron build doesn't work because of this silent error.

@akosyakov akosyakov added the dependencies pull requests that update a dependency file label Nov 13, 2019
lmcbout
lmcbout approved these changes Nov 15, 2019
View reviewed changes
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems good

@paul-marechal
Copy link
Member Author

@kittaakos change is minimal, is it good for you too? :)

@paul-marechal
Make yarn:electron fail explicitly on Windows
064fc51 
Before this commit, the `yarn rebuild:electron` used to fail silently.
We now get a proper output and exit code.

Signed-off-by: Paul Maréchal <paul.marechal@ericsson.com>
@kittaakos
Copy link
Contributor

Is this PR somehow related to #6529?

@kittaakos
Copy link
Contributor

@kittaakos change is minimal, is it good for you too? :)

I can see the error on yarn electron:rebuild.

I think we should merge it sooner than later.

Why do not we fix the error and merge with this additional check?

@paul-marechal
Copy link
Member Author

If you know how to fix the 403 error then please do. I don't know how, besides waiting for electron rebuild next release.

@kittaakos
Copy link
Contributor

If you know how to fix the 403 error then please do. I don't know how, besides waiting for electron rebuild next release.

No, I do not know either.

@paul-marechal
Copy link
Member Author

Will merge then :)

@kittaakos
Copy link
Contributor

Will merge then :)

👍 Please go ahead. Do not know why is it so urgent to merge without the fix. What will be the difference?

@paul-marechal paul-marechal merged commit a5419e3 into master Nov 19, 2019
@paul-marechal paul-marechal deleted the mp/electron-rebuild branch November 19, 2019 14:43
@paul-marechal
Copy link
Member Author

The difference will be that people will see the actual error causing Windows Electron apps to fail, and not see a consequent failure and not understand.

I think this node-gyp issue could be fixed via resolutions in dependents, just like what happened with the different vscode packages? Maybe.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmcbout lmcbout lmcbout approved these changes

@kittaakos kittaakos Awaiting requested review from kittaakos

Assignees
No one assigned
Labels
dependencies pull requests that update a dependency file
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@paul-marechal @kittaakos @lmcbout @akosyakov